Process and resolve pull request comments #18235
Merged
+27
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: burak.kaya <burak.kaya@sentry.io>
BYK
merged commit 6ea8d90
into
feat/spotlight-environment-variable-support
46 checks passed
BYK
deleted the
cursor/process-and-resolve-pull-request-comments-1f87
branch
Contributor
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (typeof import.meta !== 'undefined' && import.meta.env) { | ||
| // @ts-expect-error import.meta.env is typed differently in different environments | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| const value = import.meta.env[key]; |
There was a problem hiding this comment.
Bug: Vite: Dynamic Env Var Access Not Supported
The dynamic access import.meta.env[key] won't work in Vite-based frameworks. Vite performs static replacement at build time for literal property accesses like import.meta.env.VITE_VAR, but doesn't create a runtime object that supports bracket notation with variables. This means environment variables in Vite, Astro, and SvelteKit will never be retrieved despite the PR's intent to fix this compatibility issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).This PR addresses critical feedback regarding environment variable detection, particularly for Vite-based frameworks.
Key Changes:
packages/browser/src/utils/env.ts: ThegetEnvValuefunction now checks bothprocess.env(for Webpack, Next.js, CRA) andimport.meta.env(for Vite, Astro, SvelteKit). This ensures that environment variables (like those for Spotlight) are correctly detected across a wider range of bundlers and frameworks, fixing a significant compatibility issue.packages/browser/test/utils/env.test.ts: Updated unit tests to focus onprocess.envscenarios. Added a note explaining thatimport.meta.envcannot be unit tested due to its read-only, compile-time nature and is covered by e2e tests.packages/browser/src/utils/spotlightConfig.ts: Added a comment to clarify the explicitreturn undefinedfor readability, noting its optimization in production builds.